[FRR] Support NHG information encoding and send to fpmsyncd for RIB/FIB - #26486
[FRR] Support NHG information encoding and send to fpmsyncd for RIB/FIB#26486GaladrielZhao wants to merge 6 commits into
Conversation
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azpw ms_conflict |
3 similar comments
|
/azpw ms_conflict |
|
/azpw ms_conflict |
|
/azpw ms_conflict |
|
/azpw ms_conflict |
3 similar comments
|
/azpw ms_conflict |
|
/azpw ms_conflict |
|
/azpw ms_conflict |
e059db1 to
508a0f9
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azpw ms_conflict |
3 similar comments
|
/azpw ms_conflict |
|
/azpw ms_conflict |
|
/azpw ms_conflict |
508a0f9 to
4365ae5
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
6f5d005 to
2d5c63c
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
2d5c63c to
565c6d3
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
eddieruan-alibaba
moved this from In Progress
to routing-review-needed
in SONiC Routing Dashboard
b123f21 to
b9d52b7
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@venkit-nexthop can you help to review it again? @GaladrielZhao has already addressed all your comments. |
Use sonic-fib's Data Schema to encode zebra nexthop information
as JSON string and send it to fpmsyncd as part of the RIB/FIB project.
The modifications are in dplane_fpm_sonic.c and supervisord.conf.j2,
and can be divided into four logical parts.
1 zebra info -> C_NextHopGroupFull -> C++ NextHopGroupFull -> JSON string
Add feature to build C_NextHopGroupFull (defined in sonic-fib),
and call sonic-fib's C-API to construct C++ NextHopGroupFull,
then return the JSON string back to zebra.
2 Encode JSON string as message and send to fpmsyncd:
Implement the encoding function and modify the fpm_nl_enqueue's
DPLANE_OP_NH_XXXX case to call the new one.
3 Add DEVICE_METADATA config to enable RIB/FIB feature
Read DEVICE_METADATA.localhost.nhg_fib from ConfigDB to conditionally
pass the --nhg-fib flag to zebra at startup via supervisord.conf.j2,
enabling the RIB/FIB NHG full encoding.
4 vtysh command to control FIB log level at runtime
Add `fpm fib-log-level <debug|info|warn|error>` command to dynamically
adjust the FIB library log level, bridging fib::LogLevel in sonic-fib
with zebra's native logging system.
Signed-off-by: Yuqing Zhao <galadriel.zyq@alibaba-inc.com>
Signed-off-by: Yuqing Zhao <galadriel.zyq@alibaba-inc.com>
Signed-off-by: Yuqing Zhao <galadriel.zyq@alibaba-inc.com>
Signed-off-by: Yuqing Zhao <galadriel.zyq@alibaba-inc.com>
Signed-off-by: Yuqing Zhao <galadriel.zyq@alibaba-inc.com>
….common.j2 Signed-off-by: Yuqing Zhao <galadriel.zyq@alibaba-inc.com>
b9d52b7 to
cbfb354
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
|
||
| /* set nh_grp_full_list */ | ||
| const struct nh_grp_full *nh_grp_full_list = dplane_ctx_get_nhe_nh_grp_full(ctx); | ||
| for (uint32_t i = 0; i < dplane_ctx_get_nhe_nh_grp_full_count(ctx); i++) { |
There was a problem hiding this comment.
Here we copy nh_grp_full_list / depends / dependents without checking against the array sizes on the C_NextHopGroupFull side. Zebra already has overflow checks — we should probably do the same here so we don't write past the end of the struct.
| * For recursive NH, we keep the nexthop information for convergence handling | ||
| */ | ||
| if (CHECK_FLAG(c_nhg->nhg_flags, NEXTHOP_GROUP_RECURSIVE)) { | ||
| const struct nexthop *nh = nhg->nexthop; |
There was a problem hiding this comment.
For recursive NHGs we use nhg->nexthop without a NULL check. If that group ever comes through empty, zebra will crash. A quick guard would be good.
| * We can't use FRR's zlog here because no API for forwarding va_list, and | ||
| * we want to preserve the original log level and formatting as much as possible. | ||
| */ | ||
| fprintf(stderr, "[ZEBRA:FIB] %s:%d (%s) ", file, line, func); |
There was a problem hiding this comment.
The FIB log path is writing straight to fprintf(stderr). I understand the va_list limitation with zlog, but this will bypass the normal FRR log file / syslog setup and be annoying to operate. Could we format into a buffer and then call zlog_debug?
| /* Get the default SRv6 context which contains seg6's src IP */ | ||
| struct zebra_srv6 *srv6 = zebra_srv6_get_default(); | ||
|
|
||
| c_nhg->nh_srv6 = malloc(sizeof(struct C_nexthop_srv6)); |
There was a problem hiding this comment.
If malloc for nh_srv6 / seg6_segs fails, we keep going with a partial object and still try to encode JSON. Better to fail the encode and mark the dplane request as failed in that case.
| 0114-Provide-interface-when-installing-uninstalling-SRv6-uA-SIDs.patch | ||
| 0115-zebra-Update-promiscuity-flag-silently-without-route.patch | ||
| 0116-zebra-keep-route_node-lock-in-NB-RIB-route-lookup_next.patch | ||
| 0117-Add-Dplane-definitions-and-functions-for-ribfib.patch |
There was a problem hiding this comment.
The embedded 0117 patch is only a subset of FRR#21415, and it already diverges a bit (OPTION_NHG_FIB is 2005 here vs 2004 upstream because of kernel-mac-ext-learn). How are we planning to keep these in sync as 21415 moves?
| FPM_SRV6_LOCALSID_ACTION_UDT46 = 21, | ||
| }; | ||
|
|
||
| enum custoem_rtattr_nexthop_group { |
There was a problem hiding this comment.
Small nits while I'm here: custoem_rtattr_nexthop_group and is_recurisve look like typos. Also FPM_NHA_JSON_STR = 2 with nothing at 1 — intentional so we leave room for NHA_ID?
8 participants
Why I did it
This is one of the RIB/FIB PRs, to support sending enough NHG information to construct FIB in fpmsyncd.
How I did it
Use sonic-fib's Data Schema to encode zebra nexthop information as JSON string and send it to fpmsyncd as part of the RIB/FIB project. The modifications are in dplane_fpm_sonic.c and supervisord.conf.j2, and can be divided into four logical parts.
1 zebra info -> C_NextHopGroupFull -> C++ NextHopGroupFull -> JSON string
Add feature to build C_NextHopGroupFull (defined in sonic-fib),
and call sonic-fib's C-API to construct C++ NextHopGroupFull,
then return the JSON string back to zebra.
2 Encode JSON string as message and send to fpmsyncd:
Implement the encoding function and modify the fpm_nl_enqueue's
DPLANE_OP_NH_XXXX case to call the new one.
3 Add DEVICE_METADATA config to enable RIB/FIB feature
Read DEVICE_METADATA.localhost.nhg_fib from ConfigDB to conditionally
pass the --nhg-fib flag to zebra at startup via supervisord.conf.j2,
enabling the RIB/FIB NHG full encoding.
4 vtysh command to control FIB log level at runtime
Add
fpm fib-log-level <debug|info|warn|error>command to dynamicallyadjust the FIB library log level, bridging fib::LogLevel in sonic-fib
with zebra's native logging system.
How to verify it
A picture of a cute animal (not mandatory but encouraged)